Skip to content

Conversation

@jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Jan 23, 2026

Move constructors for string-like variants from terraform provider to helpers.go for reuse in other applications.

h/t @lgfa29.

Move constructors for string-like variants from terraform provider to
helpers.go for reuse in other applications.

h/t @lgfa29.
@jmcarp jmcarp requested a review from a team as a code owner January 23, 2026 17:49
Copy link
Member

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I was thinking a bit more about this, and I was wondering if we could use the same functional options pattern we use in the client to generate constructors for more complex variants, but I'm not sure f it would be worth it.

@sudomateo
Copy link
Collaborator

LGTM!

I was thinking a bit more about this, and I was wondering if we could use the same functional options pattern we use in the client to generate constructors for more complex variants, but I'm not sure f it would be worth it.

Like this?

rt, err := NewRouteTarget(WithTypeSubnet(/* Concrete struct here */))

@lgfa29
Copy link
Member

lgfa29 commented Jan 23, 2026

More like:

rt, err := NewRouteTarget(RouteTargetTypeIp, WithRouteTargetValue("..."))

In this case it would be kind of silly because all variants have the same shape, but in a more complex struct, where different variants take different values, it could make sense:

v1, err := NewComplexEnum(ComplexEnumType1, 
    WithComplexEnumAttr1("I'm a string"), 
    WithComplexEnumAttr2(12),
)
// v1 is:
// ComplexEnum {
//     Value: ComplexEnumType1 {
//         Attr1: "I'm a string",
//         Attr2: 12,
//     }
// }


v2, err := NewComplexEnum(ComplexEnumType2, 
    WithComplexEnumAttr1("I'm a string"), 
    WithComplexEnumAttr3(true),
)
// v2 is:
// ComplexEnum {
//     Value: ComplexEnumType2 {
//         Attr1: "I'm a string",
//         Attr3: true,
//     }
// }

It has the same downsides of any functional options approach, with the added downside of requiring users to know which types match which option functions 😅

@sudomateo
Copy link
Collaborator

It has the same downsides of any functional options approach, with the added downside of requiring users to know which types match which option functions 😅

Haha. I feel that. That's why I do believe something that takes the entire concrete struct wouldn't be too bad, but at that point might as well eliminate a constructor and trust the user to set a field directly on the wrapper struct.

Thanks for sharing the examples! It helped me understand the approach. I think I agree that moving towards functional options for this is likely not worth it overall. We can, and should, revisit this if needed.

@jmcarp jmcarp merged commit ded8773 into main Jan 27, 2026
1 check passed
@jmcarp jmcarp deleted the jmcarp/string-helpers branch January 27, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants